feat: provide de-duplicated secondary resources stream on Context#3141
feat: provide de-duplicated secondary resources stream on Context#3141
Conversation
...-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java
Show resolved
Hide resolved
...-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java
Outdated
Show resolved
Hide resolved
...mework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java
Outdated
Show resolved
Hide resolved
csviri
left a comment
There was a problem hiding this comment.
Made some minor comments, pls add unit tests! Otherwise looks great, thank you!!
|
|
||
| default <R> Stream<R> getSecondaryResourcesAsStream(Class<R> expectedType) { | ||
| return getSecondaryResources(expectedType).stream(); | ||
| return getSecondaryResourcesAsStream(expectedType, false); |
There was a problem hiding this comment.
Here we could use the former implementation and call this in case in the new method the deduplication is false.
...-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java
Show resolved
Hide resolved
...-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java
Show resolved
Hide resolved
fdfaa63 to
3c9de5c
Compare
| } | ||
| return stream | ||
| .collect( | ||
| Collectors.toUnmodifiableMap( |
There was a problem hiding this comment.
In the end, I couldn't make the implementation work without collecting to Map, I iterated over the code and ended collecting manually…
afa2dc8 to
7158917
Compare
csviri
left a comment
There was a problem hiding this comment.
Added some nits, pls make the CI happy, otherwise LGTM!
|
|
||
| /** | ||
| * Whether this ResourceID points to the same resource as the one identified by the specified name | ||
| * and namespace. Note that this doesn't take API version or Kind into account so this should only |
There was a problem hiding this comment.
nit: does not take Group either.
There was a problem hiding this comment.
Yeah, didn't want to get into all the details 😅
Actually planning on adding this to HasMetadata in the client.
| var updatedResource = | ||
| extension.get(LatestDistinctTestResource.class, TEST_RESOURCE_NAME); | ||
| assertThat(updatedResource.getStatus()).isNotNull(); | ||
| // Should see 3 distinct ConfigMaps |
There was a problem hiding this comment.
This comment needs ro be adjusted
Signed-off-by: Chris Laprun <metacosm@gmail.com>
7158917 to
a128bd7
Compare
Alternative to #3130, to discuss